On the invalid_grant concurrent-rotation retry path the write-back used the pre-retry credential for the non-token fields; thread the credential whose token produced the grant through so a concurrent re-deposit is not partially reverted. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A 403 is a permission/scope failure that a token refresh or re-consent cannot resolve, so it no longer triggers the refresh + re-auth flow (which looped) and instead surfaces as an ordinary failure. The spec specifies retry on 401. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

On the invalid_grant concurrent-rotation retry path the write-back used the pre-retry credential for the non-token fields; thread the credential whose token produced the grant through so a concurrent re-deposit is not partially reverted. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A 403 is a permission/scope failure that a token refresh or re-consent cannot resolve, so it no longer triggers the refresh + re-auth flow (which looped) and instead surfaces as an ordinary failure. The spec specifies retry on 401. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

On the invalid_grant concurrent-rotation retry path the write-back used the pre-retry credential for the non-token fields; thread the credential whose token produced the grant through so a concurrent re-deposit is not partially reverted. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A 403 is a permission/scope failure that a token refresh or re-consent cannot resolve, so it no longer triggers the refresh + re-auth flow (which looped) and instead surfaces as an ordinary failure. The spec specifies retry on 401. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

On the invalid_grant concurrent-rotation retry path the write-back used the pre-retry credential for the non-token fields; thread the credential whose token produced the grant through so a concurrent re-deposit is not partially reverted. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A 403 is a permission/scope failure that a token refresh or re-consent cannot resolve, so it no longer triggers the refresh + re-auth flow (which looped) and instead surfaces as an ordinary failure. The spec specifies retry on 401. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

On the invalid_grant concurrent-rotation retry path the write-back used the pre-retry credential for the non-token fields; thread the credential whose token produced the grant through so a concurrent re-deposit is not partially reverted. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A 403 is a permission/scope failure that a token refresh or re-consent cannot resolve, so it no longer triggers the refresh + re-auth flow (which looped) and instead surfaces as an ordinary failure. The spec specifies retry on 401. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

On the invalid_grant concurrent-rotation retry path the write-back used the pre-retry credential for the non-token fields; thread the credential whose token produced the grant through so a concurrent re-deposit is not partially reverted. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A 403 is a permission/scope failure that a token refresh or re-consent cannot resolve, so it no longer triggers the refresh + re-auth flow (which looped) and instead surfaces as an ordinary failure. The spec specifies retry on 401. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…PRD-367] (#1665)

* feat(workflow-executor): use stored OAuth credentials for MCP steps

At an oauth2 MCP step the executor looks up the stored credential by (user, server), runs the refresh-token grant against the stored token endpoint behind an expiry-skew cache, injects the bearer token before connecting, retries once on a 401 across list-tools and the tool call, and pauses for re-authentication when no usable credential exists or the refresh is rejected. Bearer and none steps are unchanged. Adds additive auth-error classification to the shared ai-proxy McpClient consumed by this path. Behaviour stays dormant until the orchestrator serves authType and the frontend ships (deploy orchestrator first), so it is safe to deploy alone. Depends on the PR1 credential store.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): write rotated refresh token to the current row

On the invalid_grant concurrent-rotation retry path the write-back used the pre-retry credential for the non-token fields; thread the credential whose token produced the grant through so a concurrent re-deposit is not partially reverted. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ai-proxy): treat only 401 (not 403) as a refreshable MCP auth error

A 403 is a permission/scope failure that a token refresh or re-consent cannot resolve, so it no longer triggers the refresh + re-auth flow (which looped) and instead surfaces as an ordinary failure. The spec specifies retry on 401. Addresses review on #1665.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(workflow-executor): support OAuth MCP steps in the in-memory executor [PRD-367]

PR1 wired an in-memory credential store + deposit endpoint into buildInMemoryExecutor, so the previous "in-memory raises ConfigurationError for oauth2 steps" behavior was inconsistent: a credential could be deposited but never used. Wire an OAuthTokenService into the in-memory runner (sharing the same store instance the deposit endpoint writes to) so oauth2 steps work end-to-end in dev, matching the database executor.

The token service is now a required RunnerConfig/RemoteToolFetcher collaborator (both executors provide it), so the unreachable ConfigurationError guard and its fetcher test are removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): repoint token service at the relocated credentials port [PRD-367]

The PR1 rebase moved the credentials store interface + types from stores/mcp-oauth-credentials-store to ports/mcp-oauth-credentials-store (the store file now holds only the Database/InMemory implementations). Import McpOAuthCredentialsStore and StoredMcpOAuthCredential from the new port path so the package compiles against the rebased PR1 base.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(workflow-executor): re-consent on decrypt failure after key rotation [PRD-367]

PR1 dropped enc_key_version from the credential store (no version-aware decrypt path), so the rotation write-back no longer carries encKeyVersion. Per PRD-367 key-rotation handling, a decrypt failure with the encryption key PRESENT (auth-tag mismatch from a since-rotated/hard-swapped key) is recoverable: toGrantParams now classifies it as OAuthReauthRequiredError (needs-oauth-reauth) so re-consent re-deposits under the new key. A missing key (ExecutorEncryptionKeyMissingError) stays terminal — re-consent cannot help and a re-deposit would 503.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(workflow-executor): add GET /list-mcp-tools design-time tool listing [PRD-367]

New GET /list-mcp-tools?mcpServerId= route on the executor HTTP server for the orchestrator-engine MCP-server details page: resolves the caller's vault credential (user_id from the validated JWT, never the request), refreshes, injects the Bearer, and returns the server's tool definitions — reusing RemoteToolFetcher, no new fetch/refresh logic. A missing/unrefreshable credential returns a typed needs-oauth-reauth (409), not a generic error or empty list. Wired into both the database and in-memory executors so oauth2 tool listing works in dev too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): guard against non-object token-endpoint responses [PRD-367]

A literal JSON null (or other non-object) body from the token endpoint overwrote the {} parse default, so the subsequent payload.error / payload.access_token reads threw a TypeError instead of the typed OAuthRefreshError. Keep the {} default for non-object bodies so the status checks still surface a typed OAuthRefreshError.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): evict cached access token on credential disconnect [PRD-367]

On DELETE /mcp-oauth-credentials/:mcpServerId, evict the in-process cached access
token for (user, server) so a disconnect takes effect immediately — otherwise the
executor keeps serving the cached token until it expires even though the credential
row is gone (surfaced by end-to-end testing).

Wires OAuthTokenService into both executor builders + the HTTP server (optional on
the options so credential-free tests need not construct one), and adds
OAuthTokenService.evict with coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workflow-executor): address PR review on oauth2 runtime [PRD-624]

- list-mcp-tools: map ExecutorEncryptionKeyMissingError to the typed 503
  { code } the deposit endpoint already returns, so the details page shows
  the admin message instead of a generic error.
- token-service: encrypt the rotated refresh token inside the best-effort
  write-back try, so an encrypt failure can't fail an otherwise-valid
  getAccessToken.
- ai-proxy isMcpAuthError: an explicit status is authoritative — a 403 is
  not refreshable even when its message says "unauthorized"; fall back to
  the message only when no status is present.
- Trim comments to why-not-how.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(workflow-executor): correct write-back-failure comment [PRD-624]

The re-read recovery path only handles a peer instance's rotation, not our
own failed write-back (the stored token is unchanged, so re-read sees no
rotation and forces re-auth). Describe the actual fallback: re-authentication.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workflow-executor): validate OAuth token endpoint against SSRF [PRD-624]

The token endpoint is where the executor POSTs the refresh grant (with
client credentials), but it was stored as an unconstrained string, so an
authenticated caller could aim the executor at an internal address (SSRF,
incl. cloud metadata).

Add assertSafeTokenEndpoint, enforced at deposit (400) and again before the
refresh POST (defence in depth for pre-existing rows):
- scheme must be http(s); https required in production (http stays allowed
  off-prod for the local OAuth sim);
- link-local / cloud-metadata (169.254.0.0/16, fe80::/10) blocked everywhere;
- loopback blocked in production;
- RFC1918 private ranges stay allowed — private OAuth providers are supported.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workflow-executor): block IPv4-mapped IPv6 in token-endpoint guard [PRD-624]

The URL parser normalizes `::ffff:127.0.0.1` to the hex form `::ffff:7f00:1`,
which the IPv6 branch classified as neither loopback nor link-local — so a
mapped loopback/metadata address slipped past the SSRF block in production.
Extract the embedded IPv4 from `::ffff:` addresses and run it through the
IPv4 loopback/link-local checks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(workflow-executor): use US spelling "defense" in refresh-grant comment [PRD-624]

Match the codebase convention (run-to-available-step-mapper.ts).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(workflow-executor): rewrite token-endpoint guard comment [PRD-624]

Drop the local-test-tool and ticket references; explain the why (SSRF
rationale, the deliberate RFC1918 allowance, why no DNS resolution) rather
than enumerating the rules.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workflow-executor): block the unspecified address in token-endpoint guard [PRD-624]

0.0.0.0 (and ::) connects to loopback on Linux, so it was an SSRF bypass the
guard accepted. Classify the unspecified range (0.0.0.0/8, ::) and reject it
on every environment — it is never a valid token endpoint.

Surfaced by evaluating ipaddr.js, which flags it as `unspecified`; the WHATWG
URL parser already normalizes the decimal/hex/short-form IP encodings to
dotted form, so those were already covered.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workflow-executor): reject localhost FQDN/.localhost aliases in token guard [PRD-624]

Only the bare `localhost` was treated as loopback, so `localhost.` and the
`.localhost` TLD (RFC 6761) — both resolving to loopback — slipped past the
guard in production. Match any host whose last label is `localhost` (with or
without a trailing dot), without over-blocking real domains like
auth.localhost.example.com.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(workflow-executor): cover trailing-dot IP forms in token-endpoint guard [PRD-624]

Regression test proving `127.0.0.1.` / `169.254.169.254.` are rejected: the
guard runs `new URL()` first, which normalizes the trailing dot before
classification, so the trailing-dot form is not a bypass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workflow-executor): address PR review — SSRF redirect, deposit evict, fail-closed env [PRD-624]

- refresh-grant: reject redirects on the token POST (redirect:'manual' + any 3xx →
  OAuthRefreshError) so a validated token endpoint can't 3xx the grant body (refresh
  token / client secret) to an internal host, bypassing the endpoint validation.
- executor-http-server: evict the cached access token on deposit too, so a reconnect /
  re-deposit takes effect immediately instead of serving the prior token until expiry
  (mirrors the delete path).
- token-endpoint-url: fail closed — the strict https/no-loopback rules now apply unless
  NODE_ENV explicitly opts into the dev/test relaxation, so an unset or misspelled
  NODE_ENV can no longer silently allow loopback targets or cleartext http.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(workflow-executor): harden OAuth reauth and token write-back [PRD-692] (#1724)

* fix(workflow-executor): harden OAuth reauth and token write-back

Three correctness fixes that gate the OAuth2 MCP executor flag:
- clear the step idempotency marker on a re-auth pause so a resumed step
  is no longer rejected as interrupted
- record a re-auth pause as a non-failure in the activity log instead of
  a spurious failure
- re-check credential existence before the rotated-token write-back so a
  concurrent disconnect is not silently resurrected

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(workflow-executor): strengthen reauth-pause audit assertion

Assert the activity entry is closed as succeeded on a re-auth pause, not
just that it was never failed. Drop the stale TDD/iteration framing and
ticket references from the re-auth pause test block.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): make OAuth refresh write-back atomic

Replace the get-then-upsert existence re-check with an atomic
updateIfPresent (UPDATE ... WHERE) on the credentials store, closing the
read-to-write window where a concurrent disconnect could be resurrected.
upsert stays for the consent deposit path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): audit a re-auth-paused MCP call as failed

Revert the isNonFailure audit special-casing on the re-auth pause path.
An MCP tool call that 401s has genuinely failed, so it should surface as
a failed audit entry (useful for observability) rather than be recorded
as completed. The step still pauses (awaiting-input) and the resumed run
logs its own entry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): correct reauth-pause cleanup and write-back scope

Addresses review findings on the re-auth pause and refresh write-back:
- preserve a confirmation-flow step's approved pendingData on a re-auth
  pause (clear only the marker) so resume replays that exact call;
  delete only when there is no pendingData (FullyAutomated), which would
  otherwise mis-route the resumed step into the confirmation flow
- make the pause cleanup best-effort so a store error still returns
  awaiting-input instead of a hard failure
- key updateIfPresent on the row id so a disconnect + re-authorize is not
  clobbered by a stale in-flight write-back (a re-created row has a new id)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(workflow-executor): trim added comments to the 2-line convention

Keep the non-obvious why (preserve-vs-delete on re-auth pause, id-scoped write-back); drop what the method names and code already state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(workflow-executor): propagate reauth-cleanup store errors

Swallowing a cleanup failure left the 'executing' marker in place, so the pause returned awaiting-input but could never resume (checkIdempotency rejects the stale marker). Let the store error propagate to an ordinary step error instead — consistent with the executor's other store-failure paths. Supersedes the earlier best-effort try/catch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(workflow-executor): link the OAuth2-MCP test server

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>